Skip to content

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Apr 1, 2025

We need the bucket interval within the rate function to perform extrapolation. With this change, along with #26089, we will be able to pass the grouping range interval to rate aggregations.

@dnhatn dnhatn force-pushed the add-range-to-rate branch 4 times, most recently from e975a0e to 1d9543d Compare April 3, 2025 20:09
@dnhatn dnhatn changed the title Add bucket interval to time-series rate aggregation Add time bucket to time-series rate aggregation Apr 3, 2025
@dnhatn dnhatn force-pushed the add-range-to-rate branch from 1d9543d to 2746d98 Compare April 3, 2025 21:17
@dnhatn dnhatn force-pushed the add-range-to-rate branch from 2746d98 to e972f91 Compare April 3, 2025 21:22
@dnhatn dnhatn requested review from kkrik-es and martijnvg April 3, 2025 21:22
@dnhatn dnhatn marked this pull request as ready for review April 3, 2025 21:22
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:StorageEngine labels Apr 3, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)


public TimeSeriesAggregate(StreamInput in) throws IOException {
super(in);
this.timeBucket = in.readOptionalWriteable(inp -> (Bucket) Bucket.ENTRY.reader.read(inp));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be versioned?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disabled time_series tests in mixed clusters to allow us to move quickly here.

Copy link
Contributor

@kkrik-es kkrik-es left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for now. We'll eventually want to pass the interval as an arg to rate and agg_over_time functions, or as a second arg to TBUCKET, but we can look at that later.

@dnhatn
Copy link
Member Author

dnhatn commented Apr 4, 2025

Thanks Kostas!

@dnhatn dnhatn merged commit 6ce8d51 into elastic:main Apr 4, 2025
18 checks passed
@dnhatn dnhatn deleted the add-range-to-rate branch April 4, 2025 06:53
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM2

andreidan pushed a commit to andreidan/elasticsearch that referenced this pull request Apr 9, 2025
We need the bucket interval within the rate function to perform 
extrapolation. With this change, along with elastic#26089, we will be able to
pass the grouping range interval to rate aggregations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >non-issue :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:StorageEngine v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants